-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make initial StateTransition
run before PreStartup
#14208
Conversation
I just tested this and it looks like it works 👍🏻 |
I haven't looked at the code, but: IMO this approach is fine for 0.14.1, but a state disabling / enabling approach would be preferable for 0.15. |
I've thought about it, a little bit If we do introduce |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running a schedule within another (commonly-used) bevy-provided schedule, without even exposing a system set or the system itself to order around, is problematic -- this is a clear improvement over the current behavior, and makes the code simpler.
No reason not to merge afaic. Better solution can be implemented later.
world: &mut World, | ||
startup_label: Option<InternedScheduleLabel>, | ||
) { | ||
pub fn setup_state_transitions_in_world(world: &mut World) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function signature change is a semver violation by Rust's rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with this design, and I think that the technical semver violation isn't important enough to worry about. I would be very surprised to see users calling that method themselves, even though it is pub.
# Objective - Fixes #14206 ## Solution - Run initial `StateTransition` as a startup schedule before `PreStartup`, instead of running it inside `Startup` as an exclusive system. Related discord discussion: https://discord.com/channels/691052431525675048/692572690833473578/1259543775668207678 ## Testing Reproduction now works correctly: ```rs use bevy::prelude::*; #[derive(Debug, Clone, Copy, Default, Eq, PartialEq, Hash, States)] enum AppState { #[default] Menu, InGame, } fn main() { App::new() .add_plugins(DefaultPlugins) .init_state::<AppState>() .add_systems(Startup, setup) .add_systems(OnEnter(AppState::Menu), enter_menu_state) .run(); } fn setup(mut next_state: ResMut<NextState<AppState>>) { next_state.set(AppState::Menu); } fn enter_menu_state() { println!("Entered menu state"); } ``` ![image](https://github.com/bevyengine/bevy/assets/13040204/96d7a533-c439-4c0b-8f15-49f620903ce1) --- ## Changelog - Initial `StateTransition` runs before `PreStartup` instead of inside `Startup`.
Objective
OnEnter
schedule is not ran when callingnext_state.set()
inStartup
system #14206Solution
StateTransition
as a startup schedule beforePreStartup
, instead of running it insideStartup
as an exclusive system.Related discord discussion:
https://discord.com/channels/691052431525675048/692572690833473578/1259543775668207678
Testing
Reproduction now works correctly:
Changelog
StateTransition
runs beforePreStartup
instead of insideStartup
.